Skip to content

Conversation

@danieldegrasse
Copy link
Contributor

@danieldegrasse danieldegrasse commented Sep 28, 2022

Enable relocation of network subsystem to RAM. The intent of this feature is to enable SOCs with fast instruction TCM to take advantage of the improved performance available by relocating networking code to this region. This PR enables this feature for the Cortex M7 based iMX RT SOCs with a Kinetis Ethernet controller.

This PR is dependent on the following PRs (and currently integrates the commits from both):
#50791
#50790

Below are some TCP performance results using this PR. Results were obtained using iperf 2.1.5 with the zperf sample running on the Zephyr target. These results relocate the networking stack, L2 ethernet implementation, and NXP ethernet driver to ITCM.

Board SHA TCP RX TCP TX
mimxrt1060_evk main (fd9e542) 44.2Mbps 43.7Mbps
mimxrt1060_evk This PR 94.5Mbps 71.6Mbps
mimxrt1050_evk main (fd9e542) 60.2Mbps 68.5Mbps
mimxrt1050_evk This PR 94.5Mbps 71.4Mbps

@zephyrbot zephyrbot added manifest west manifest-hal_nxp DNM This PR should not be merged (Do Not Merge) labels Sep 28, 2022
@zephyrbot
Copy link

The following west manifest projects have been modified in this Pull Request:

Name Old Revision New Revision Diff
hal_nxp zephyrproject-rtos/hal_nxp@708c958 (master) zephyrproject-rtos/hal_nxp#193 zephyrproject-rtos/hal_nxp#193/files

Note: This message is automatically posted and updated by the Manifest GitHub Action.

@jukkar
Copy link
Member

jukkar commented Sep 29, 2022

Shouldn't this be a board feature and not a networking one? I am asking because you introduce new Kconfig symbols to networking stack which imho do not seem to be really needed there.

@jukkar
Copy link
Member

jukkar commented Sep 29, 2022

The feature itself is nice if we get such a speed increase with minimal changes. Just pondering where / how this should be setup.

@danieldegrasse
Copy link
Contributor Author

@jukkar I'm open to making this a board or SOC feature, but the issue then becomes how to relocate the networking library. Currently, the zephyr_library_relocate macro in #50791 is used, which would need to be in the network CMakeLists.txt. Maybe we could use another macro, like zephyr_library_relocate_named, to allow the SOC or board CMakeLists to relocate networking code?

@tbursztyka
Copy link
Contributor

Looks fine to me to get a generic config option on net stack, that some hw can take advantage of. It's actually cleaner that way I think. Many more SoC/boards are going to use that at some point

Copy link
Contributor

@tbursztyka tbursztyka Sep 29, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

commit title would need to be revised, you are relocating both the net stack and the ethernet L2. And there is no dependency with each other on this. Perhaps splitting the patch in 2 would be clearer: first the net stack, then the ethernet L2.

Add zephyr_library_relocate, a helper macro intended to allow entire
Zephyr libraries to be relocated to a new memory region.

This is implemented via a two step process, to ensure that sources added
to a library after relocation is requested still are relocated:
1. zephyr_library_relocate adds a custom target property to the library
   with the relocation region and flags
2. the root level CMakeLists.txt captures this property while parsing all
   Zephyr libraries, and adds all library source files to the code
   relocation target

Signed-off-by: Daniel DeGrasse <[email protected]>
…ocate

The build script parsing files that will be relocated using the
zephyr_code_relocate macro was adding KEEP() to all symbols.

This resulted in unnecessary symbols being added to the build, and could
result in a linker failure if symbols in the relocated file that the
linker would otherwise discard referenced undefined symbols.

This error was observed while relocating portions of the networking
subsystem to SRAM, since the network management API was not being compiled
but some unused functions in the network subsystem were using the API.

Signed-off-by: Daniel DeGrasse <[email protected]>
Introduce CONFIG_NET_CODE_RELOCATE, to relocate Zephyr networking
subsystem to RAM. This RAM region defaults to SRAM, but platforms can
change the default region based on what memory is present.

Signed-off-by: Daniel DeGrasse <[email protected]>
Change the default target RAM for the network stack code to be relocated
to to ITCM for NXP Cortex M7 platforms using the Kinetis Ethernet
peripheral. This will enable improved performance over the default
relocation region of SRAM.

Signed-off-by: Daniel DeGrasse <[email protected]>
Enable code relocation for RT1050 and RT1060

Signed-off-by: Daniel DeGrasse <[email protected]>
@danieldegrasse
Copy link
Contributor Author

@tbursztyka I've updated this PR to only relocate the networking subsystem to RAM (and select the appropriate target RAM for NXP i.MX parts). Performance numbers have been updated as well, and are surprisingly very similar to relocating the L2 as well as driver code.

I'm going to remove the draft status of this PR, as based on initial feedback it seems this direction may be acceptable (@jukkar, feel free to let me know if you'd strongly prefer to move this feature out of the subsystem. I feel that the relocation being done at the subsystem level makes the feature more generic, but I am open to feedback)

@danieldegrasse danieldegrasse changed the title net: allow relocation of network subsystem to RAM [DNM] net: allow relocation of network subsystem to RAM Sep 29, 2022
@jukkar
Copy link
Member

jukkar commented Sep 30, 2022

@danieldegrasse I have no strong opinion about this, I was just thinking aloud and pondering about how to setup this. Overall, this is a nice speed improvement for this soc.

@github-actions
Copy link

github-actions bot commented Jan 2, 2023

This pull request has been marked as stale because it has been open (more than) 60 days with no activity. Remove the stale label or add a comment saying that you would like to have the label removed otherwise this pull request will automatically be closed in 14 days. Note, that you can always re-open a closed pull request at any time.

@github-actions github-actions bot added the Stale label Jan 2, 2023
@github-actions github-actions bot closed this Jan 17, 2023
@dleach02 dleach02 deleted the feature/network_subsys_relocate branch July 17, 2024 01:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants